Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add mkdir_p to FileUtils.install #104

Merged
merged 6 commits into from
Feb 10, 2023
Merged

Add mkdir_p to FileUtils.install #104

merged 6 commits into from
Feb 10, 2023

Conversation

satmandu
Copy link
Contributor

Fixes #103

  • PR to solve the aforementioned issue of FileUtils.install not creating a destination directory before installing a file to a destination directory.
  • Using mkdir_p on the destination directory solves that issue.

uberhacker
uberhacker reviewed Jan 10, 2023
View reviewed changes
uberhacker
uberhacker reviewed Jan 10, 2023
View reviewed changes
supechicken
supechicken reviewed Jan 11, 2023
View reviewed changes
@satmandu satmandu requested review from uberhacker and supechicken and removed request for uberhacker and supechicken January 11, 2023 17:04
supechicken
supechicken reviewed Jan 11, 2023
View reviewed changes
@satmandu
Copy link
Contributor Author

Is this acceptable @hsbt ?

@hsbt
Copy link
Member

hsbt commented Jan 17, 2023

Thanks for your work. At least, We need to the additional tests. Can you add them?

@satmandu
Copy link
Contributor Author

Thanks for your work. At least, We need to the additional tests. Can you add them?

What are the additional needed tests?

@satmandu
Copy link
Contributor Author

@hsbt What additional tests are needed?

@hsbt
Copy link
Member

hsbt commented Jan 19, 2023

for your changes.

@satmandu
Copy link
Contributor Author

Ah! I will modify https://github.com/ruby/fileutils/blob/master/test/fileutils/test_fileutils.rb ...

@satmandu
Copy link
Contributor Author

Does this work?

@hsbt
Copy link
Member

hsbt commented Jan 20, 2023

I'll fix this CI fails. It's not related with your tests.

@satmandu
Copy link
Contributor Author

@hsbt Any chance of this getting merged?

@hsbt hsbt merged commit 8af16ba into ruby:master Feb 10, 2023
matzbot pushed a commit to ruby/ruby that referenced this pull request Feb 10, 2023
@satmandu @matzbot
[ruby/fileutils] Add mkdir_p to FileUtils.install
94aed6e 
(ruby/fileutils#104)

* Add mkdir_p to FileUtils.install

* Adjust raise message.

* adjust raise language

* handle trailing slash in dest

* simplify

* Add tests
@satmandu satmandu deleted the install_mkdir_p branch February 10, 2023 03:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@uberhacker uberhacker uberhacker left review comments

@supechicken supechicken supechicken left review comments

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

FileUtils.install needs integrated mkdir_p
4 participants
@satmandu @hsbt @uberhacker @supechicken